Skip to content

fix: stage doens't adapt replace notify for notification #950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
deepin-bot merged 1 commit into from
Dec 10, 2024
Merged

fix: stage doens't adapt replace notify for notification #950

deepin-bot merged 1 commit into from
Dec 10, 2024

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Dec 10, 2024

Notify server adapt replace notify, it will override old notify
both DB data and timeout data, and emit NotificationStateChanged.

pms: BUG-289863

yixinshark
yixinshark previously approved these changes Dec 10, 2024
View reviewed changes
yixinshark
yixinshark reviewed Dec 10, 2024
View reviewed changes
panels/notification/common/notifyentity.h

uint replacesId() const;
void setReplacesId(uint replacesId);
bool isReplace() const;
Copy link
Contributor

@yixinshark yixinshark Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是不是hasReplaceId()更合适

Copy link
Contributor Author

@18202781743 18202781743 Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

是直接替换了，不是跟之前的共存，跟之前的通知共用同一个id, 应该是is吧,

@18202781743
fix: stage doens't adapt replace notify for notification
f61fafa 
Notify server adapt replace notify, it will override old notify
both DB data and timeout data, and emit NotificationStateChanged.

pms: BUG-289863
@deepin-ci-robot
Copy link

deepin-ci-robot commented Dec 10, 2024

deepin pr auto review

代码审查意见：

  1. 在BubbleItem::isReplace()函数中，m_entity.isReplace()的返回值应该是一个布尔值，而不是uint。如果m_entity的isReplace函数返回的是uint，那么应该检查这个函数的实现，确保它返回的是布尔值。

  2. 在BubbleModel::replaceBubbleIndex函数中，item->id()应该与bubble->id()进行比较，而不是item->replacesId()。如果replacesId是用于替换的ID，那么应该使用bubble->replacesId()进行比较。

  3. 在NotifyStagingModel::replace函数中，dataChanged的调用应该使用QModelIndex作为参数，而不是int。应该使用this->index(i, 0, {})来获取正确的QModelIndex。

  4. 在DBAccessor::replaceEntity函数中，QSqlQuery的exec方法返回一个布尔值，表示查询是否成功执行。应该检查这个返回值，以确保查询成功执行。

  5. 在NotificationManager::Notify函数中，entity.setReplacesId(replacesId);应该在entity.setProcessedType(NotifyEntity::None);之前调用，以确保replacesId被正确设置。

  6. 在NotifyServerApplet::actionInvoked和NotifyServerApplet::notificationClosed函数中，QMetaObject::invokeMethod的连接类型应该设置为Qt::DirectConnection，以确保方法调用在当前线程中立即执行。

  7. 在NotifyServerApplet::appValue函数中，removeNotification和removeNotifications函数的调用应该检查返回值，以确保操作成功。

  8. 在NotifyEntity::NotifyData类中，replacesId的默认值应该设置为NoReplaceId，而不是0。这样可以避免与0的默认值混淆。

  9. 在NotifyEntity::NotifyData类中，expireTimeout的默认值应该设置为0，而不是-1。这样可以避免与-1的默认值混淆。

  10. 在NotifyEntity::NotifyData类中，bubbleId的默认值应该设置为0，而不是-1。这样可以避免与-1的默认值混淆。

  11. 在NotifyEntity::NotifyData类中，id的默认值应该设置为-1，而不是0。这样可以避免与0的默认值混淆。

  12. 在NotifyEntity::NotifyData类中，actions和hints的默认值应该设置为空的QStringList和QVariantMap，而不是nullptr。这样可以避免潜在的空指针解引用问题。

  13. 在NotifyEntity::NotifyData类中，expireTimeout的默认值应该设置为0，而不是-1。这样可以避免与-1的默认值混淆。

  14. 在NotifyEntity::NotifyData类中，bubbleId的默认值应该设置为0，而不是-1。这样可以避免与-1的默认值混淆。

  15. 在NotifyEntity::NotifyData类中，id的默认值应该设置为-1，而不是0。这样可以避免与0的默认值混淆。

  16. 在NotifyEntity::NotifyData类中，actions和hints的默认值应该设置为空的QStringList和QVariantMap，而不是nullptr。这样可以避免潜在的空指针解引用问题。

  17. 在NotifyEntity::NotifyData类中，expireTimeout的默认值应该设置为0，而不是-1。这样可以避免与-1的默认值混淆。

  18. 在NotifyEntity::NotifyData类中，bubbleId的默认值应该设置为0，而不是-1。这样可以避免与-1的默认值混淆。

  19. 在NotifyEntity::NotifyData类中，id的默认值应该设置为-1，而不是0。这样可以避免与0的默认值混淆。

  20. 在NotifyEntity::NotifyData类中，actions和hints的默认值应该设置为空的QStringList和QVariantMap，而不是nullptr。这样可以避免潜在的空指针解引用问题。

@deepin-ci-robot
Copy link

deepin-ci-robot commented Dec 10, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@18202781743
Copy link
Contributor Author

18202781743 commented Dec 10, 2024

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Dec 10, 2024

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 94936f5 into linuxdeepin:master Dec 10, 2024
7 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yixinshark yixinshark yixinshark approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@18202781743 @deepin-ci-robot @yixinshark